feat(#816): make retro agent multi-forge (GitHub + GitLab) by fullsend-ai-coder[bot] · Pull Request #817 · fullsend-ai/agents

fullsend-ai-coder · 2026-08-15T13:59:03Z

Summary

Add GitLab support to the retro agent, following the three-file lib pattern (retro-ops.lib.sh dispatcher + github-retro-ops.lib.sh + gitlab-retro-ops.lib.sh) established by the triage agent
Convert pre-retro.sh and post-retro.sh to .src.sh source files that call forge_* abstraction functions instead of hardcoded gh CLI commands
Split policies, env files, and skills into forge-specific subdirectories; update harness/retro.yaml with forge.gitlab section; relax target_repo schema pattern for GitLab subgroups

Testing

All 36 existing GitHub tests pass with FULLSEND_FORGE=github
Added 10 GitLab tests: happy path (no gh calls), issue creation via curl, evidence-for filtering, subgroup repo paths, URL validation/rejection, allowed/disallowed hosts, comment truncation limit (1MB vs 65K)
make check-bundle passes — bundled .sh files match .src.sh source
shellcheck passes on all new and modified shell scripts
hack/lint-agent-docs passes — harness doc references valid

Closes #816

Post-script verification

Branch is not main/master (agent/816-retro-multi-forge)
Secret scan passed (gitleaks — 693ed83bbd5289e1ef13d6c7a3772f61efc43c86..HEAD)
PR body secret scan passed (gitleaks — no-git)

fullsend-ai-review · 2026-08-15T14:00:10Z

🤖 Finished Review · ✅ Success · Started 2:00 PM UTC · Completed 2:16 PM UTC

Commit: 2003e4d · View workflow run →

fullsend-ai-review · 2026-08-15T14:16:12Z

Review

Findings

Medium

[protected-path] agents/retro.md, harness/retro.yaml, policies/gitlab/retro.yaml, scripts/lib/*.lib.sh, scripts/post-retro.sh, scripts/post-retro.src.sh, scripts/pre-retro.sh, scripts/pre-retro.src.sh, skills/retro-analysis/*, skills/finding-agent-runs/* — This PR modifies 17 files under protected paths (agents/, harness/, policies/, scripts/, skills/). The PR links to issue feat: make retro agent multi-forge (GitHub + GitLab) #816 and the description explains the rationale for adding GitLab support to the retro agent. Human approval is always required for protected-path changes, regardless of context.

Low

[code-organization] harness/retro.yaml — The forge.github and forge.gitlab sections omit pre_script/post_script entries, relying on the top-level declarations. Other multi-forge harnesses are inconsistent about this (triage/code/fix include both levels; review uses forge-level only). Not a functional issue, but a minor style inconsistency.

Info

[provenance-warning] — Prior review context discarded: provenance validation failed (unverifiable-wrong-app). This review treats all findings as first-time assessments.

Previous run

Review

Findings

Medium

[GHA workflow command injection] scripts/lib/gitlab-retro-ops.lib.sh:122 — In forge_create_issue(), the error path echoes unsanitized curl output: echo "GitLab API error: issue creation failed in ${repo}: ${response}". The ${response} variable contains raw curl output which could contain GHA workflow commands (::set-env, ::add-mask, etc.) when running in a GitHub Actions context. _gha_sanitize is available and used consistently elsewhere in the codebase for this purpose.
Remediation: Wrap ${response} in _gha_sanitize().
[architectural-coherence] harness/retro.yaml:32 — pre_script and post_script are defined at the top level only (lines 32–33) and not in the forge.github or forge.gitlab sections. All other multi-forge harnesses (triage.yaml, code.yaml, review.yaml) define pre_script/post_script in each forge section. The base-branch retro.yaml had these in forge.github, but this PR removed them. If the harness runtime requires forge-section scripts (rather than inheriting from top-level), the pre/post scripts would not run for forge-specific invocations.
Remediation: Add pre_script: scripts/pre-retro.sh and post_script: scripts/post-retro.sh to both the forge.github and forge.gitlab sections.
[write-capable token in sandbox] harness/retro.yaml — GITLAB_TOKEN is passed into the sandbox environment (forge.gitlab.env.sandbox.GITLAB_TOKEN). The policy at policies/gitlab/retro.yaml acknowledges the write-capable token is present and relies on access: read-only network policy enforcement. This is a defense-in-depth concern: the GitHub forge avoids the analogous risk by excluding curl from binaries entirely.
Remediation: Consider provisioning a read-only scoped GitLab token (read_api scope only) for sandbox use.
[protected-path] This PR modifies 17 files under protected paths (agents/, harness/, policies/, scripts/, skills/). The PR is linked to issue feat: make retro agent multi-forge (GitHub + GitLab) #816 which provides detailed rationale for the multi-forge retro agent implementation. Human approval is required for protected-path changes regardless of context.

Low

[naming-convention] scripts/lib/retro-ops.lib.sh:14 — The forge_* function prefix is consistent with review-ops, code-ops, and prioritize-ops dispatchers, but inconsistent with triage-ops which uses tracker_*. The inconsistency is pre-existing and not introduced by this PR; the retro implementation follows the majority convention.

Next steps:

/fs-fix — agent addresses review findings automatically
/fs-fix <your instruction> — agent fixes with your specific guidance
Push commits directly — review re-runs automatically on push
/fs-fix-stop — disable automatic fix runs for this PR

Previous run (2)

Review

Findings

Medium

[protected-path] agents/retro.md, harness/retro.yaml, policies/gitlab/retro.yaml, scripts/lib/*.lib.sh, scripts/post-retro*.sh, scripts/pre-retro*.sh, skills/finding-agent-runs/**, skills/retro-analysis/** — 17 of 23 changed files are under protected paths (agents/, harness/, policies/, scripts/, skills/). The PR links to issue feat: make retro agent multi-forge (GitHub + GitLab) #816 and explains the rationale for adding GitLab support to the retro agent. Human approval is always required for protected-path changes, regardless of context.
[Secrets in sandbox] harness/retro.yaml — The write-capable GITLAB_TOKEN is passed into the sandbox environment. The network policy (policies/gitlab/retro.yaml) enforces access: read-only on gitlab_api endpoints, so the sandboxed agent cannot perform write mutations. This matches the established pattern for all other GitLab agents and is an accepted architectural risk, not a regression introduced by this PR.

Low

[error-message-idiom] scripts/lib/retro-ops.lib.sh:21 — Error message for invalid FULLSEND_FORGE uses different phrasing than the other dispatch wrappers (code-ops.lib.sh, review-ops.lib.sh, prioritize-ops.lib.sh), which all include pass --forge <github|gitlab> or set FULLSEND_FORGE. The retro variant says only set FULLSEND_FORGE to github or gitlab.
[function-signature-consistency] scripts/lib/github-retro-ops.lib.sh:51 — forge_create_label takes an explicit repo parameter (4 args) while every other agent's forge_create_label uses a global REPO variable (3 args). This divergence is deliberate — the retro agent files issues to multiple target repos — but undocumented. A header comment noting the intentional difference would prevent future confusion.
[naming-convention] scripts/lib/gitlab-retro-ops.lib.sh:37 — Uses GITLAB_TOKEN/GH_TOKEN directly instead of the <AGENT>_TOKEN naming pattern (e.g., REVIEW_TOKEN, TRIAGE_TOKEN) used by other agents. Internally consistent across both forges in this PR.
[scope-deviation] harness/retro.yaml — Top-level pre_script/post_script and skills are retained alongside forge sections, deviating from the issue feat: make retro agent multi-forge (GitHub + GitLab) #816 plan but following the established triage.yaml convention.

Info

[provenance-warning] — Prior review context discarded: provenance validation failed (unverifiable-wrong-app). This review treats all findings as first-time assessments.

Previous run (3)

Review

Findings

Medium

[pattern-violation] harness/retro.yaml — The pre_script and post_script directives appear only at the top level (lines 32–33), not within the forge.github or forge.gitlab sections. Every other multi-forge agent (code, review, triage, fix) duplicates pre_script/post_script inside each forge block. The PR removed them from forge.github and did not add them to forge.gitlab. If the harness runtime does not fall back to top-level values when a forge section is active, the pre/post scripts will not execute. Consider adding pre_script: scripts/pre-retro.sh and post_script: scripts/post-retro.sh inside both forge blocks to match the established pattern.
[protected-path] harness/retro.yaml, agents/retro.md, policies/gitlab/retro.yaml, scripts/* (7 files), skills/* (6 files) — This PR modifies files under protected paths. The PR links to issue feat: make retro agent multi-forge (GitHub + GitLab) #816 and the description explains the rationale (multi-forge support for the retro agent). Human approval is always required for protected-path changes, regardless of context.

Low

[naming-convention] scripts/lib/retro-ops.lib.sh — The error message in the catch-all case says "pass --forge <github|gitlab> or set FULLSEND_FORGE" but the retro agent scripts have no --forge CLI flag (unlike pre-triage.src.sh which accepts --tracker). Consider changing to "set FULLSEND_FORGE to github or gitlab".
[new-required-env-var] harness/retro.yaml — A new required env var FULLSEND_FORGE is introduced. Both pre-retro.sh and post-retro.sh fail immediately if this var is absent. This is consistent with how other agents (code, review, triage) handle forge dispatch, and any runtime already running those agents supports forge sections.
[edge-case] scripts/lib/gitlab-retro-ops.lib.sh:24 — The GitLab URL validation regex requires at least two path segments after the host ({2,}), rejecting single-segment GitLab namespaces. This is intentional per the test suite (gl-rejects-single-segment-url) and represents a documented constraint.
[error-handling] scripts/lib/gitlab-retro-ops.lib.sh:100 — In forge_create_issue, when the GitLab API call fails, the error message is echoed to stdout and captured by the caller, producing a doubled "failed to create issue" prefix in the final error output. Cosmetically redundant but does not affect error handling logic.
[error-handling-idiom] scripts/lib/gitlab-retro-ops.lib.sh — Error messages in forge_create_issue go to stdout instead of stderr. The established pattern in gitlab-triage-ops.lib.sh sends error messages to stderr with >&2. Note: the current behavior is load-bearing — the caller uses command substitution to capture output, so changing to stderr would also require adjusting the caller logic.

Previous run (4)

Review

Findings

Medium

[scope-gap] agents/retro.md — Issue feat: make retro agent multi-forge (GitHub + GitLab) #816 authorizes adding FULLSEND_FORGE to the Inputs section of agents/retro.md, but the PR omits this addition. The agent receives FULLSEND_FORGE in the sandbox env (via forge.<platform>.env.sandbox) and the ops lib dispatches on it (retro-ops.lib.sh). Other multi-forge agents (agents/review.md) document this input in their Inputs section.
Remediation: Add FULLSEND_FORGE to the Inputs list in agents/retro.md.
[protected-path] agents/, harness/, policies/, scripts/, skills/ — This PR modifies 17 files under protected paths. The PR links to issue feat: make retro agent multi-forge (GitHub + GitLab) #816 and the description explains the rationale (adding GitLab support to the retro agent following the established multi-forge pattern). Human approval is always required for protected-path changes, regardless of context.

Low

[test-adequacy] scripts/post-retro-test.sh — The GitLab test suite does not include a test for the FULLSEND_VALIDATED_ITERATION_DIR code path. The GitHub test section includes a dedicated run_validated_dir_test block, but no GitLab equivalent exists. The code path is forge-agnostic, so the risk is minimal.
[naming-convention] harness/retro.yaml — The forge-specific host_files destination uses forge-retro.env, matching code.yaml's forge-code.env naming but diverging from the majority pattern (review.yaml, triage.yaml, prioritize.yaml) which omit the forge- prefix.
[error-handling-idiom] scripts/lib/gitlab-retro-ops.lib.sh — The _gitlab_api helper uses GITLAB_TOKEN directly, whereas gitlab-review-ops.lib.sh uses REVIEW_TOKEN (a forge-normalized alias). The retro agent is internally consistent but diverges from the review agent's abstraction pattern.

Previous run (5)

Review

Findings

Medium

[protected-path] agents/retro.md, harness/retro.yaml, policies/gitlab/retro.yaml, scripts/lib/*-retro-ops.lib.sh, scripts/post-retro*.sh, scripts/pre-retro*.sh, skills/retro-analysis/**, skills/finding-agent-runs/** — 17 files in this PR modify protected paths (agents/, harness/, policies/, scripts/, skills/). The PR links to issue feat: make retro agent multi-forge (GitHub + GitLab) #816 and the description explains the rationale for these changes. Human approval is always required for protected-path changes, regardless of context.
[secrets-handling] policies/gitlab/retro.yaml:12 — Write-capable GITLAB_TOKEN is present inside the sandbox with only network-policy access: read-only enforcement as the write guard. Unlike the GitHub forge where curl is excluded from the binary allowlist as defense-in-depth, GitLab requires curl for API access. If access: read-only enforcement has a bypass, the full-write token is available to the sandboxed agent. Consider issuing a read-only GitLab token (read_api scope) for the sandbox environment, keeping the write-capable token only for the host post-script.

Low

[code-organization] harness/retro.yaml — The retro harness declares pre_script/post_script only at the top level, while every other multi-forge agent (code, triage, fix, review) declares them inside each forge.<platform> section. The diff shows these being removed from forge.github. This diverges from the established pattern; verify that the fullsend CLI correctly inherits top-level declarations for all forge sections.
[GHA-workflow-command-injection] scripts/post-retro.src.sh:135 — When target_repo fails validation, unsanitized TR (agent output) is echoed to stderr without calling _gha_sanitize. The function is available and used elsewhere in the same script for similar purposes. Remediation: echo "ERROR: ... $(_gha_sanitize "${TR}")" >&2
[code-organization] Makefile:4 — BUNDLE_SRCS entries for retro are appended after triage, breaking the alphabetical-by-agent ordering (code, fix, prioritize, review, triage). Move retro between review and triage to match the order used in FEATURES.md.

Previous run (6)

Review

Findings

Medium

[protected-path] agents/retro.md, harness/retro.yaml, policies/gitlab/retro.yaml, scripts/lib/github-retro-ops.lib.sh, scripts/lib/gitlab-retro-ops.lib.sh, scripts/lib/retro-ops.lib.sh, scripts/post-retro-test.sh, scripts/post-retro.sh, scripts/post-retro.src.sh, scripts/pre-retro.sh, scripts/pre-retro.src.sh, skills/finding-agent-runs/SKILL.md, skills/finding-agent-runs/github/SKILL.md, skills/finding-agent-runs/gitlab/SKILL.md, skills/retro-analysis/SKILL.md, skills/retro-analysis/github/SKILL.md, skills/retro-analysis/gitlab/SKILL.md — 17 of 20 changed files fall under protected paths (agents/, harness/, policies/, scripts/, skills/). The PR links to issue feat: make retro agent multi-forge (GitHub + GitLab) #816 and the description explains the rationale for each change. Human approval is always required for protected-path changes, regardless of context.
[scope-gap] harness/retro.yaml — Issue feat: make retro agent multi-forge (GitHub + GitLab) #816 explicitly lists env/github/retro.env and env/gitlab/retro.env under files to create, with host_files entries in the forge sections. Every other multi-forge agent (triage, code, review, prioritize) has forge-specific env files referenced via host_files entries. This PR omits both files and their host_files entries, instead passing env vars directly through the harness env: blocks. Consider either creating the env files to match the established pattern, or noting in the PR description why the retro agent does not need them.

Low

[harness config structure] harness/retro.yaml — The forge.github and forge.gitlab sections omit pre_script and post_script keys. While triage, code, review, and fix repeat these inside each forge section, the prioritize agent also omits them — so the convention is split, not universal. Consider adding them for consistency with the majority pattern.
[incomplete-doc-list] FEATURES.md:94 — The generated scripts checklist enumerates all bundled .sh scripts but omits scripts/pre-retro.sh and scripts/post-retro.sh, which this PR converts from hand-written to generated scripts (via .src.sh sources added to BUNDLE_SRCS in the Makefile).

Previous run (7)

Review

Findings

Medium

[secrets-in-sandbox] harness/retro.yaml:80 — Write-capable GITLAB_TOKEN is injected into the sandbox environment. The policy enforces read-only network access on GitLab API endpoints, and curl is necessarily allowed (GitLab has no read-only CLI equivalent). If the access:read-only enforcement has a bypass, the agent would gain write access. Consider using a read-only scoped GitLab token for the sandbox to match the defense-in-depth pattern used by the GitHub forge.
[incomplete-doc] docs/retro.md:52 — Variables section says "None." but FULLSEND_FORGE is now required by both pre-retro.sh and post-retro.sh. Other multi-forge agent docs (code, review, prioritize, triage) document FULLSEND_FORGE in a Variables table.

Low

[scope-deviation] harness/retro.yaml — Missing forge-specific env files (env/github/retro.env, env/gitlab/retro.env) and host_files entries that every other multi-forge agent has. Functionally equivalent via harness env blocks, but diverges from the established pattern.
[code-organization] harness/retro.yaml:33 — pre_script/post_script defined only at top level. Triage and code harnesses have them in both top level and forge sections; review has them only in forge sections. Three valid patterns exist in the codebase.
[env-var-contract] harness/retro.yaml:68 — New mandatory FULLSEND_FORGE env var, consistent with all other multi-forge agents. Harness sets it in forge-specific env blocks.
[env-var-contract] scripts/post-retro.src.sh:8 — GH_TOKEN requirement deferred to forge_require_token(). Expected for multi-forge — FULLSEND_FORGE is checked first, then the forge-appropriate token.
[incomplete-doc] docs/retro.md:65 — No "Multi-forge support" documentation section. Code and triage agents include one, but review also omits it.
[error-handling] scripts/lib/gitlab-retro-ops.lib.sh:146 — forge_post_comment error handler uses local rc=$? which deviates from the codebase convention of splitting the local declaration from the $? assignment.
[naming-convention] scripts/lib/gitlab-retro-ops.lib.sh:30 — Uses GITLAB_TOKEN directly for authentication, while gitlab-review-ops.lib.sh uses REVIEW_TOKEN (a harness-level alias). Different implicit contracts across GitLab ops libs.
[incomplete-doc] docs/network-policy.md:102 — Note mentions only the code agent having a GitLab policy under policies/gitlab/. Retro (and triage, review) also have one.
[schema-backward-compatibility] schemas/retro-result.schema.json:30 — target_repo pattern relaxed from two to two-or-more segments. Backward compatible; needed for GitLab subgroup paths.
[script-output-contract] scripts/post-retro.src.sh:129 — Error message changed from "is not a valid owner/repo" to "is not a valid repo path".

Previous run (8)

Review

Findings

Medium

[protected-path] agents/, harness/, policies/, scripts/, skills/ — This PR modifies 18 files under protected governance and infrastructure paths. The linked issue (feat: make retro agent multi-forge (GitHub + GitLab) #816) and PR description explain the rationale (multi-forge support following the established triage-agent pattern). Human approval is required for all protected-path changes regardless of context.

Low

[edge-case] scripts/lib/gitlab-retro-ops.lib.sh:138 — forge_post_comment uses the global ORIGINATING_RESOURCE (set by forge_parse_originating_url) to determine the API path (/issues/N/notes vs /merge_requests/N/notes). The function signature accepts repo and number as parameters but not resource type, creating implicit coupling. Currently safe — called only once for the originating entity — but would silently produce incorrect API calls if reused for a different entity type.
[sanitization-gap] scripts/lib/retro-ops.lib.sh:11 — _gha_sanitize() handles ANSI escape stripping. Edge case: the ANSI regex covers CSI sequences (\x1b[...) but not OSC (\x1b]) or other less common escape types. Negligible gap — :: and %0A/newline protections are the primary GHA command injection controls.
[input-validation] scripts/lib/gitlab-retro-ops.lib.sh:39 — forge_validate_originating_url() regex character class [a-zA-Z0-9._-] for the hostname segment accepts single-label hostnames. Mitigated by the subsequent case statement allowlist at line 45 (gitlab.com|gitlab.cee.redhat.com), which rejects any host not in the list.
[secrets-handling] policies/gitlab/retro.yaml — The GitLab retro sandbox policy enforces access: read-only on gitlab_api endpoints. The write-capable GITLAB_TOKEN is present in the sandbox, consistent with the existing GH_TOKEN pattern for GitHub. Write protection relies on the sandbox enforcement layer's access: read-only constraint.
[naming-conventions] scripts/lib/gitlab-retro-ops.lib.sh:22 — Private API wrapper function is named _gitlab_api(), but the code agent's equivalent uses _gitlab_code_api(). For consistency across agents, consider renaming to _gitlab_retro_api().

Previous run (9)

Review

Findings

Medium

[protected-path] agents/, harness/, policies/, scripts/, skills/ — This PR modifies 18 files under protected governance and infrastructure paths. The linked issue (feat: make retro agent multi-forge (GitHub + GitLab) #816) and PR description explain the rationale (multi-forge support following the established triage-agent pattern). Human approval is required for all protected-path changes regardless of context.

Low

[edge-case] scripts/lib/gitlab-retro-ops.lib.sh:138 — forge_post_comment uses the global ORIGINATING_RESOURCE (set by forge_parse_originating_url) to determine the API path (/issues/N/notes vs /merge_requests/N/notes). The function signature accepts repo and number as parameters but not resource type, creating implicit coupling. Currently safe — called only once for the originating entity — but would silently produce incorrect API calls if reused for a different entity type.
[sanitization-gap] scripts/lib/retro-ops.lib.sh:11 — _gha_sanitize() handles ANSI escape stripping (resolving the prior finding). Edge case: the ANSI regex covers CSI sequences (\x1b[...) but not OSC (\x1b]) or other less common escape types. Negligible gap — :: and %0A/newline protections are the primary GHA command injection controls.
[input-validation] scripts/lib/gitlab-retro-ops.lib.sh:41 — forge_validate_originating_url() regex character class [a-zA-Z0-9._-] for the hostname segment accepts single-label hostnames. Mitigated by the subsequent case statement allowlist at line 47 (gitlab.com|gitlab.cee.redhat.com), which rejects any host not in the list.
[secrets-handling] harness/retro.yaml:74 — GITLAB_TOKEN is passed to both env.runner (line 74) and env.sandbox (line 79) for the GitLab forge. Consistent with the existing GH_TOKEN pattern for the GitHub forge. The GitLab sandbox policy enforces access: read-only on gitlab_api endpoints.
[secrets-handling] scripts/lib/gitlab-retro-ops.lib.sh:74 — forge_mask_token() is a no-op when GITHUB_ACTIONS is not set (i.e., on GitLab CI runners). The guard correctly prevents ::add-mask:: from echoing the token as plaintext. GitLab CI's own variable masking must be relied upon instead.
[naming-conventions] scripts/lib/gitlab-retro-ops.lib.sh:22 — _gitlab_api uses underscore prefix to indicate private scope, but the GitHub counterpart does not define an equivalent _github_api function. The asymmetry stems from different API access patterns (curl wrapper vs gh CLI).
[error-handling] scripts/lib/gitlab-retro-ops.lib.sh:103 — Label creation error suppression in forge_create_label uses a different pattern than the GitHub counterpart. Both achieve idempotent label creation; the patterns differ due to API differences (gh --force vs curl error suppression).
[schema-relaxation] schemas/retro-result.schema.json:31 — The target_repo pattern relaxation from ^[a-zA-Z0-9._-]+/[a-zA-Z0-9._-]+$ (exactly two segments) to ^[a-zA-Z0-9._-]+(/[a-zA-Z0-9._-]+)+$ (two or more segments) is backward compatible for GitHub paths and appropriate for GitLab subgroups. The restrictive character class prevents injection. Both schema and post-script validation regex are updated atomically.

Previous run (10)

Review

Findings

Medium

[protected-path] agents/, harness/, policies/, scripts/, skills/ — This PR modifies 18 files under protected governance and infrastructure paths. The linked issue (feat: make retro agent multi-forge (GitHub + GitLab) #816) and PR description explain the rationale (multi-forge support following the established triage-agent pattern). Human approval is required for all protected-path changes regardless of context.
[error-handling-gap] scripts/lib/gitlab-retro-ops.lib.sh:123 — forge_create_issue pipes the _gitlab_api response through jq -r '.web_url' without guarding against a null result. If the GitLab API returns HTTP 200 with valid JSON that lacks a web_url field (e.g., an API version mismatch or unexpected response shape), jq outputs the string "null" with exit code 0. The caller then prints Created: null and proceeds as if the issue was created successfully, producing a broken link in the summary comment. The GitHub counterpart (gh issue create) validates the response internally and avoids this path.
Remediation: After extracting web_url with jq, add a guard: local url; url=$(echo "${response}" | jq -r '.web_url'); if [[ -z "${url}" || "${url}" == "null" ]]; then echo "GitLab API error: unexpected response from issue creation in ${repo}"; return 1; fi; echo "${url}"

Low

[sanitization-gap] scripts/post-retro.src.sh:162 — The SAFE_TITLE sanitization in the evidence-for rejection block (lines 162–168) and the issue-filing block (lines 206–210) strips :: and percent-encoded newlines but does not strip ANSI escape codes, unlike _gha_sanitize() used elsewhere in the same script (lines 265, 270). SAFE_TITLE is interpolated into ::warning:: GHA workflow commands. Low practical impact — title text from jq extraction is unlikely to contain ANSI codes, and GHA log rendering does not interpret ANSI escapes in workflow commands — but the inconsistency is worth addressing.
[input-validation] scripts/lib/gitlab-retro-ops.lib.sh:39 — The forge_validate_originating_url() regex character class [a-zA-Z0-9._-] for the hostname segment accepts single-label hostnames (e.g., https://gitlab/-/issues/1 would pass the regex). Mitigated by the subsequent case statement allowlist at line 46 (gitlab.com|gitlab.cee.redhat.com), which rejects any host not in the list.
[secrets-handling] harness/retro.yaml:74 — GITLAB_TOKEN is passed to both env.runner (line 74) and env.sandbox (line 79) for the GitLab forge. Consistent with the existing GH_TOKEN pattern for the GitHub forge. The GitLab sandbox policy enforces access: read-only on gitlab_api endpoints.
[secrets-handling] scripts/lib/gitlab-retro-ops.lib.sh:74 — forge_mask_token() is a no-op when GITHUB_ACTIONS is not set (i.e., on GitLab CI runners). GitLab CI's own variable masking (CI/CD variable settings with "masked" flag) must be relied upon instead. The token is not echoed directly by any script path, so practical exposure risk is low.
[edge-case] scripts/lib/gitlab-retro-ops.lib.sh:132 — forge_post_comment uses the global ORIGINATING_RESOURCE (set by forge_parse_originating_url) to determine the API path (/issues/ or /merge_requests/). This implicit coupling is currently safe — the function is called exactly once, always for the originating repo — but would silently produce incorrect API calls if reused for a different context.
[schema-relaxation] schemas/retro-result.schema.json:31 — The target_repo pattern relaxation from ^[a-zA-Z0-9._-]+/[a-zA-Z0-9._-]+$ (exactly two segments) to ^[a-zA-Z0-9._-]+(/[a-zA-Z0-9._-]+)+$ (two or more segments) is backward compatible for GitHub paths and appropriate for GitLab subgroups. The restrictive character class prevents injection. Both schema and post-script validation regex are updated atomically.

Next steps:

/fs-fix — agent addresses review findings automatically
/fs-fix <your instruction> — agent fixes with your specific guidance
Push commits directly — review re-runs automatically on push
/fs-fix-stop — disable automatic fix runs for this PR

Previous run (11)

Review

Findings

Medium

[protected-path] agents/, harness/, policies/, scripts/, skills/ — This PR modifies 18 files under protected governance and infrastructure paths. The linked issue (feat: make retro agent multi-forge (GitHub + GitLab) #816) and PR description explain the rationale (multi-forge support following the established triage-agent pattern). Human approval is required for all protected-path changes regardless of context.

Low

[sanitization-gap] scripts/post-retro.src.sh:264 — The SAFE_OUTPUT manual sanitization strips :: and percent-encoded newlines but does not strip ANSI escape codes (unlike _gha_sanitize()). ANSI escapes in COMMENT_OUTPUT from curl/gh could obscure GHA log output, though they cannot inject workflow commands. Low practical impact.
[stale-doc] docs/retro.md:72 — Line 72 says "Post-script creates GitHub issues from the agent's proposals." The PR updated other occurrences of "GitHub issues" in this file and in agents/retro.md but missed this one. Should be "creates issues" to reflect multi-forge support.
[input-validation] scripts/lib/gitlab-retro-ops.lib.sh:33 — The forge_validate_originating_url() regex character class [a-zA-Z0-9._-] for the hostname accepts single-label hostnames. Mitigated by the subsequent case statement allowlist (gitlab.com|gitlab.cee.redhat.com).
[secrets-handling] harness/retro.yaml:75 — GITLAB_TOKEN is passed to both env.runner and env.sandbox for the GitLab forge. Consistent with existing GH_TOKEN pattern. The GitLab sandbox policy restricts network to read-only for gitlab.com and gitlab.cee.redhat.com.
[secrets-handling] scripts/lib/gitlab-retro-ops.lib.sh:69 — forge_mask_token() is a no-op when GITHUB_ACTIONS is not set (i.e., on GitLab CI runners). GitLab CI's own variable masking must be relied upon instead. Scripts do not echo the token directly, so practical exposure risk is low.
[schema-relaxation] schemas/retro-result.schema.json:31 — The target_repo pattern relaxation from two segments to two-or-more is appropriate for GitLab subgroups. The restrictive character class prevents injection. Both schema and post-script are updated atomically.
[naming-convention] env/github/retro.env:4 — GH_TOKEN is exported without quotes (export GH_TOKEN=${GH... while other exports in the same file use quoted expansion ("${VAR}"`). Minor inconsistency.

Previous run (12)

Review

Findings

Medium

[GHA-workflow-command-injection] scripts/lib/gitlab-retro-ops.lib.sh:19 — forge_mask_token() emits echo "::add-mask::${GITLAB_TOKEN}", which is a GHA-only directive. On GitLab CI runners, ::add-mask:: is not recognized, so the raw token value is printed to the CI job log in cleartext. The comment says "GHA-only; harmless no-op on other runners" but echo is not a no-op — it actively prints the token to stdout. This function is called unconditionally from post-retro.src.sh via forge_mask_token.
Remediation: Guard the emit with if [[ -n "${GITHUB_ACTIONS:-}" ]]; then echo "::add-mask::..."; fi so the token is not echoed to non-GHA runner logs, or skip masking on the GitLab path as the triage agent does.
[protected-path] agents/, harness/, policies/, scripts/, skills/ — This PR modifies 18 files under protected governance and infrastructure paths. The linked issue (feat: make retro agent multi-forge (GitHub + GitLab) #816) and PR description explain the rationale (multi-forge support following the established triage-agent pattern). Human approval is required for all protected-path changes regardless of context.

Low

[sanitization-gap] scripts/post-retro.src.sh:264 — The SAFE_OUTPUT manual sanitization strips :: and percent-encoded newlines but does not strip ANSI escape codes (unlike _gha_sanitize()). ANSI escapes in COMMENT_OUTPUT from curl/gh could obscure GHA log output, though they cannot inject workflow commands. Low practical impact.
[stale-doc] docs/retro.md:72 — Line 72 says "Post-script creates GitHub issues from the agent's proposals." The PR updated other occurrences of "GitHub issues" in this file (line 5) and in agents/retro.md but missed this one. Should be "creates issues" to reflect multi-forge support.
[stale-doc] README.md:38 — Line 38 says "runs on the GitHub Actions runner to validate inputs and prepare the environment". The PR updated nearby line 40 from "GitHub mutations" to "forge mutations" but left this reference to "GitHub Actions runner" intact, which is inconsistent with the forge-agnostic framing.
[input-validation] scripts/lib/gitlab-retro-ops.lib.sh:31 — The forge_validate_originating_url() regex character class [a-zA-Z0-9._-] for the hostname accepts single-label hostnames. Mitigated by the subsequent case statement allowlist (gitlab.com|gitlab.cee.redhat.com).
[schema-relaxation] schemas/retro-result.schema.json:31 — The target_repo pattern relaxation from two segments to two-or-more is appropriate for GitLab subgroups. The restrictive character class prevents injection. Both schema and post-script are updated atomically.
[secrets-handling] harness/retro.yaml:56 — GITLAB_TOKEN is passed to both env.runner and env.sandbox for the GitLab forge. This is consistent with the existing GH_TOKEN pattern. The GitLab sandbox policy restricts network to read-only for gitlab.com and gitlab.cee.redhat.com with binaries limited to curl and node.

Previous run (13)

Review

Findings

Medium

[GHA-workflow-command-injection] scripts/lib/gitlab-retro-ops.lib.sh:19 — forge_mask_token() emits echo "::add-mask::${GITLAB_TOKEN}", which is a GHA-only directive. On GitLab CI runners, ::add-mask:: is not recognized, so the raw token value is printed to the CI job log in cleartext. The comment says "GHA-only; harmless no-op on other runners" but echo is not a no-op — it actively prints the token to stdout. The triage agent's GitLab path avoids this entirely by not using ::add-mask:: on non-GHA runners.
Remediation: Guard the emit with if [[ -n "${GITHUB_ACTIONS:-}" ]]; then echo "::add-mask::..."; fi so the token is not echoed to non-GHA runner logs, or skip masking on the GitLab path as the triage agent does.
[protected-path] agents/, harness/, policies/, scripts/, skills/ — This PR modifies 18 files under protected governance and infrastructure paths. The linked issue (feat: make retro agent multi-forge (GitHub + GitLab) #816) and PR description explain the rationale (multi-forge support following the established triage-agent pattern). Human approval is required for all protected-path changes regardless of context.

Low

[sanitization-gap] scripts/post-retro.src.sh:264 — The SAFE_OUTPUT manual sanitization strips :: and percent-encoded newlines but does not strip ANSI escape codes (unlike _gha_sanitize()). ANSI escapes in COMMENT_OUTPUT from curl/gh could obscure GHA log output, though they cannot inject workflow commands. Low practical impact.
[stale-doc] docs/retro.md:72 — Line 72 says "Post-script creates GitHub issues from the agent's proposals." The PR updated other occurrences of "GitHub issues" in this file (line 5) and in agents/retro.md but missed this one. Should be "creates issues" to reflect multi-forge support.
[input-validation] scripts/lib/gitlab-retro-ops.lib.sh:31 — The forge_validate_originating_url() regex character class [a-zA-Z0-9._-] for the hostname accepts single-label hostnames. Mitigated by the subsequent case statement allowlist (gitlab.com|gitlab.cee.redhat.com).
[schema-relaxation] schemas/retro-result.schema.json:31 — The target_repo pattern relaxation from two segments to two-or-more is appropriate for GitLab subgroups. The restrictive character class prevents injection. Both schema and post-script are updated atomically.
[secrets-handling] harness/retro.yaml:56 — GITLAB_TOKEN is passed to both env.runner and env.sandbox for the GitLab forge. This is consistent with the existing GH_TOKEN pattern. The GitLab sandbox policy restricts network to read-only for gitlab.com and gitlab.cee.redhat.com with binaries limited to curl and node.
[naming-conventions] scripts/lib/retro-ops.lib.sh:11 — _gha_sanitize function name uses the _gha_ prefix suggesting GHA-specificity, but it is defined in the forge-agnostic wrapper file and used in both GitHub and GitLab contexts. The function's purpose IS GHA-specific (preventing GHA workflow command injection), which partially justifies the name.

Labels: PR adds GitLab multi-forge support to the retro agent, touching retro-specific scripts, harness, policies, and skills.

Next steps:

/fs-fix — agent addresses review findings automatically
/fs-fix <your instruction> — agent fixes with your specific guidance
Push commits directly — review re-runs automatically on push
/fs-fix-stop — disable automatic fix runs for this PR

Previous run (14)

Review

Findings

Medium

[error-handling-gap] scripts/post-retro.src.sh:261 — The 401/403 graceful-degradation check uses grep -qE "HTTP (401|403)", which matches the gh CLI error format but not curl --fail --show-error output (curl: (22) The requested URL returned error: 403). On the GitLab code path, a 403 from the comment-posting endpoint will fall through to the fatal error branch, aborting the script instead of degrading gracefully — even though proposals and issues have already been filed successfully. The mock curl in tests outputs the gh-compatible format, masking this divergence.
Remediation: Update the grep pattern to also match curl's format (e.g., grep -qE "HTTP (401|403)|error: (401|403)"), or use curl --write-out '%{http_code}' to capture status codes numerically.
[api-contract] scripts/lib/gitlab-retro-ops.lib.sh:108 — forge_post_comment declares local repo="$1" but ignores it, using the global ORIGINATING_REPO instead to compute repo_encoded. The GitHub counterpart correctly uses its $repo parameter. Currently the sole call site passes ORIGINATING_REPO, so behavior is correct, but the contract is misleading and will silently ignore the argument if a future caller passes a different repo.
Remediation: Change repo_encoded=$(printf '%s' "${ORIGINATING_REPO}" | jq -sRr @uri) to use ${repo} instead.
[protected-path] agents/, harness/, policies/, scripts/, skills/ — This PR modifies 18 files under protected governance and infrastructure paths. The linked issue (feat: make retro agent multi-forge (GitHub + GitLab) #816) and PR description explain the rationale (multi-forge support following the established triage-agent pattern). Human approval is required for all protected-path changes regardless of context.

Low

[test-inadequate] scripts/post-retro-test.sh — The GitLab test suite does not include comment-posting failure tests (401/403/500). The GitHub suite has comprehensive failure coverage (comment-403-non-fatal, comment-401-non-fatal, comment-500-fatal, comment-422-fatal). The run_gl_test function accepts a comment_fail parameter but is never invoked with non-empty values.
[test-inadequate] scripts/post-retro-test.sh:96 — The mock curl outputs error messages in gh-compatible format (HTTP 403: Forbidden) rather than real curl --fail --show-error format (curl: (22) The requested URL returned error: 403 Forbidden), masking the grep pattern mismatch in the graceful-degradation logic.
[schema-backward-compatibility] schemas/retro-result.schema.json:31 — The target_repo pattern relaxation (two segments → two or more) is backward compatible for GitHub paths but creates a rollout dependency: consumers using the old schema will reject GitLab subgroup paths. Both schema and post-script are updated atomically in this PR, limiting practical impact.
[stale-doc] README.md:39 — The architecture section says "perform GitHub mutations" in the post-script phase description. With the retro agent (and previously triage) now supporting GitLab, this is partially stale.

Next steps:

/fs-fix — agent addresses review findings automatically
/fs-fix <your instruction> — agent fixes with your specific guidance
Push commits directly — review re-runs automatically on push
/fs-fix-stop — disable automatic fix runs for this PR

fullsend-ai-review · 2026-08-15T14:38:05Z

🤖 Finished Review · ✅ Success · Started 2:38 PM UTC · Completed 2:59 PM UTC

Commit: cf76f9e · View workflow run →

fullsend-ai-review · 2026-08-15T15:08:22Z

🤖 Finished Review · ✅ Success · Started 3:08 PM UTC · Completed 3:26 PM UTC

Commit: a47de65 · View workflow run →

Superseded by updated review

fullsend-ai-review · 2026-08-15T19:55:08Z

🤖 Finished Review · ✅ Success · Started 7:55 PM UTC · Completed 8:10 PM UTC

Commit: 1e8a8d9 · View workflow run →

fullsend-ai-review · 2026-08-16T02:57:13Z

🤖 Review · ⚠️ Cancelled · Started 2:57 AM UTC · Ended 3:01 AM UTC

Commit: 2063a81 · View workflow run →

fullsend-ai-review · 2026-08-16T03:02:18Z

🤖 Finished Review · ✅ Success · Started 3:02 AM UTC · Completed 3:20 AM UTC

Commit: c87a124 · View workflow run →

fullsend-ai-review · 2026-08-16T12:36:59Z

🤖 Review · ⚠️ Cancelled · Started 12:36 PM UTC · Ended 12:48 PM UTC

Commit: 78564ca · View workflow run →

fullsend-ai-review · 2026-08-16T12:49:32Z

🤖 Finished Review · ✅ Success · Started 12:49 PM UTC · Completed 1:26 PM UTC

Commit: b1d1ab6 · View workflow run →

Superseded by updated review

fullsend-ai-review · 2026-08-20T16:35:48Z

🤖 Finished Review · ✅ Success · Started 4:35 PM UTC · Completed 4:53 PM UTC

Commit: aa46a0d · View workflow run →

fullsend-ai-review · 2026-08-20T17:00:43Z

🤖 Finished Review · ✅ Success · Started 5:00 PM UTC · Completed 5:20 PM UTC

Commit: 3a15805 · View workflow run →

fullsend-ai-review · 2026-08-20T17:27:19Z

🤖 Finished Review · ✅ Success · Started 5:27 PM UTC · Completed 5:45 PM UTC

Commit: 39f3988 · View workflow run →

fullsend-ai-review · 2026-08-20T19:19:28Z

🤖 Finished Review · ✅ Success · Started 7:19 PM UTC · Completed 7:41 PM UTC

Commit: 8d652b4 · View workflow run →

fullsend-ai-review · 2026-08-20T19:56:00Z

🤖 Review · ⚠️ Cancelled · Started 7:55 PM UTC · Ended 8:01 PM UTC

Commit: 98abe92 · View workflow run →

Add GitLab support to the retro agent following the three-file lib pattern established by the triage agent. The retro agent now dispatches forge-specific operations via FULLSEND_FORGE env var. - Add retro-ops.lib.sh dispatcher with github/gitlab forge ops libs - Refactor pre-retro and post-retro scripts to use forge_* functions - Split skills (retro-analysis, finding-agent-runs) into shared methodology + forge-specific CLI recipe subdirs - Add policies/gitlab/retro.yaml with read-only GitLab API access - Update harness with forge.github and forge.gitlab sections - Relax schema target_repo pattern for GitLab subgroups - Extend test suite with GitLab tests including comment failure cases, CURL_LOG endpoint verification, and URL-encoding checks Signed-off-by: Greg Allen <gallen@redhat.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Greg Allen <gallen@redhat.com>

fullsend-ai-review · 2026-08-20T20:03:29Z

🤖 Finished Review · ✅ Success · Started 8:03 PM UTC · Completed 8:22 PM UTC

Commit: 0cd0fad · View workflow run →

Superseded by updated review

ggallen · 2026-08-20T20:36:33Z

/ok-to-test

fullsend-ai-retro · 2026-08-20T20:57:57Z

🤖 Finished Retro · ✅ Success · Started 8:57 PM UTC · Completed 9:09 PM UTC

Commit: 0cd0fad · View workflow run →

fullsend-ai-retro · 2026-08-20T21:09:50Z

Retro: PR #817 — make retro agent multi-forge (GitHub + GitLab)

Timeline

Aug 15 12:52 — ggallen creates issue #816 with a detailed 5000+ word implementation spec covering 23 files, 10 implementation steps, and 5 verification criteria.
Aug 15 12:53–12:57 — Triage agent runs successfully, labels ready-to-code.
Aug 15 12:58 — ggallen triggers /fs-code.
Aug 15 12:59–13:19 — First code agent run fails — shellcheck SC2031 (subshell variable modification) and SC2034 (unused variable) warnings in generated shell scripts.
Aug 15 13:36 — ggallen retriggers /fs-code with additional guidance about established shellcheck suppression patterns (# shellcheck disable=SC2031 in test files, # shellcheck disable=SC2034 for variables consumed after bundling).
Aug 15 13:37–13:59 — Second code agent run succeeds, creates PR #817 with 23 files changed.
Aug 15 14:16 — Review round 1 (commit 2003e4d): 2 medium findings (error-handling-gap in curl grep pattern, API-contract violation in forge_post_comment) + 2 low findings.
Aug 15 14:36 — ggallen fixes both mediums in commit cf76f9e, acknowledges lows.
Aug 16 03:20 — Review round 2 (commit c87a124): 1 new medium (missing null guard on web_url extraction) + 6 low findings (5 repeated from round 1).
Aug 16 — ggallen fixes the medium in commit 78564ca, acknowledges repeated lows.
Aug 16 13:25 – Aug 20 — Review rounds 3–15: all findings are low-severity repeats of previously acknowledged observations. The same 5 findings (input-validation, secrets-handling x2, schema-relaxation, edge-case) reappeared across 6–11 of the 15 total rounds.
Aug 20 20:36 — ggallen posts /ok-to-test.
Aug 20 20:56 — PR merged (1 squashed commit).

What went well

Triage was fast and accurate — 4 minutes, correctly assessed the well-specified issue as ready-to-code.
Code agent produced a working 23-file implementation on its second attempt, following the established three-file lib pattern from the triage agent.
Review agent caught 3 genuine medium-severity bugs in the first two rounds — real error-handling gaps and an API contract violation that would have caused incorrect behavior on the GitLab code path.
Human-agent review collaboration was effective — ggallen fixed the real bugs, made informed accept/reject decisions on low-severity observations, and merged confidently.
Issue spec quality was exceptional — the 5000+ word spec with file-by-file instructions, code snippets, and verification criteria gave the code agent strong guidance.

Inefficiencies observed (all covered by existing issues)

1. Repeated review findings across 15 rounds. The same low-severity findings (input-validation, secrets-handling, schema-relaxation, edge-case coupling) were re-posted 6–11 times each, even after ggallen explicitly acknowledged them. The consolidated review comment grew to include 14 collapsed "Previous run" sections. This appears related to provenance checks discarding prior review context.

Evidence for: agents#106 (re-raises declined findings), agents#685 (persist prior-iteration findings), agents#721 (stable IDs for dedup), fullsend#2959 (dedup across iterations), fullsend#5265 (incorporate author responses), fullsend#2794 (identical protected-path comments every run).

2. Self-acknowledged findings posted as issues. The review agent posted findings where its own analysis concluded the design was intentional (e.g., "Consistent with existing GH_TOKEN pattern", "Mitigated by the subsequent case statement allowlist").

Evidence for: agents#688 (should not post self-acknowledged-as-justified observations).

3. Code agent shellcheck failure on first run. The code agent didn't know the repo's established shellcheck suppression patterns, causing a pre-commit hook failure. The human resolved this by providing the pattern references on retry.

Partially covered by: agents#286 (run verification commands pre-submit) and agents#679 (pre-validate against repo lint config).

4. Consolidated review comment bloat. With 15 review rounds, the sticky comment accumulated 14 collapsed "Previous run" sections.

Evidence for: fullsend#2358 (cap stacked historical iterations).

Conclusion

The workflow succeeded — the review agent caught real bugs, the code agent delivered a well-structured implementation, and the human made good accept/reject decisions. The primary cost driver was repeated review findings across 15 rounds, a known limitation with extensive existing issue coverage. No novel proposals are warranted.

fullsend-ai-coder Bot requested a review from a team as a code owner August 15, 2026 13:59

fullsend-ai-coder Bot added the ready-for-review label Aug 15, 2026

fullsend-ai-coder Bot assigned ggallen Aug 15, 2026